Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Nimble SwiftLint compatible #370 #392

Closed
wants to merge 1 commit into from
Closed

Make Nimble SwiftLint compatible #370 #392

wants to merge 1 commit into from

Conversation

lferro9000
Copy link

When building the project in Carthage with swiftlint installed, there are 11 warnings issued that trip the build, as reported on the #370 and using the current version available by using Carthage.

This PR solves all but one by amending the code to correct the offending issues.

The last one is fixed by adding a specific disable. This was choosen due to lack of deep knowledge of the finner details of the codebase and the fact that the affected code is underway changes on future PR that are currently WIP.

No behavior of the code was changed.

When building the project in Carthage with swiftlint installed, there are 11 warnings issued that trip the build.

This PR solves all but one by amending the code to correct the offending issues.

The last one is fixed by adding a specific disable. This was choosen due to lack of deep knowledge of the finner details of the codebase and the fact that the affected code is underway changes on future PR that are currently WIP.
@@ -51,7 +51,7 @@ final class BeIdenticalToTest: XCTestCase, XCTestCaseProvider {
func testBeAlias() {
let value = NSDate()
expect(value).to(be(value))
expect(NSNumber(value:1)).toNot(be(NSString(stringLiteral: "turtles")))
expect(NSNumber(value:1)).toNot(be(NSString(stringLiteral: "turtles"))) // swiftlint:disable:this compiler_protocol_init
Copy link
Member

@wongzigii wongzigii Feb 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this disable rule necessary? Otherwise looks good to me.

@jeffh
Copy link
Member

jeffh commented Feb 14, 2017

Hey @lferro9000,

There was a merge conflict because of recent CwlPreconditionTesting updates, but I've updated the project to SwiftLint and credited to your name.

Also, I've correctly the NSString(stringLiteral:) usage to not require suppressing the warning as well.

Here's the commit if you're interested, cheers.

@jeffh jeffh closed this Feb 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants